Skip to content

Fix compile error in mcpGatewayToolBrokerChannel test#299072

Merged
connor4312 merged 15 commits intomainfrom
connor4312/fix-mcp-gateway-compile-error
Mar 4, 2026
Merged

Fix compile error in mcpGatewayToolBrokerChannel test#299072
connor4312 merged 15 commits intomainfrom
connor4312/fix-mcp-gateway-compile-error

Conversation

@connor4312
Copy link
Member

Fixes compile error from #298040 — the Promise type parameter in createNeverStartingServer used { state: McpConnectionState.Kind } (wide enum type) instead of McpConnectionState.

RajeshKumar11 and others added 10 commits February 26, 2026 22:42
Instead of immediately returning empty results for Unknown-state servers
and refreshing in the background, wait up to 5 seconds for the server to
become live on the first list call. Subsequent calls find the already-
resolved promise in _startupGrace and return immediately, so only the
first message pays the startup cost.

- _waitForStartup: races _ensureServerReady against the configurable
  grace period (default 5 000 ms); result is cached per server
- _shouldUseCachedData: awaits the grace period for Unknown servers,
  falls back to background-refresh for Outdated servers
- _listTools: refactored to Promise.all to parallelise per-server waits
- Tests: updated 'starts server when cache state is unknown' to assert
  tools are returned after the grace period; added new test for the
  timeout path using createNeverStartingServer
Per review feedback: Outdated servers should use the same per-server
startup grace period as Unknown servers. A prior fast startup does not
guarantee a fast restart, so both states now race startup against the
5s timeout before returning results.
Copilot AI review requested due to automatic review settings March 3, 2026 22:05
@connor4312 connor4312 enabled auto-merge March 3, 2026 22:05
@vs-code-engineering vs-code-engineering bot added this to the 1.111.0 milestone Mar 3, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a compile error introduced in PR #298040 ("MCP Gateway: avoid blocking list calls on startup"). The main change is correcting the Promise type parameter in createNeverStartingServer from the overly-broad { state: McpConnectionState.Kind } to the correct McpConnectionState union type. The PR also includes the underlying feature changes from #298040: a startup grace period mechanism in McpGatewayToolBrokerChannel that races server startup against a configurable timeout, allowing cached data to be returned immediately without blocking list calls on server startup.

Changes:

  • Added a _startupGrace per-server promise cache and _waitForStartup/_shouldUseCachedData methods to McpGatewayToolBrokerChannel, replacing the direct _ensureServerReady blocking calls in _listTools, _listResources, and _listResourceTemplates
  • Fixed compile error in test: new Promise<McpConnectionState>(() => { }) instead of new Promise<{ state: McpConnectionState.Kind }>(() => { })
  • Added new test cases covering the grace period behavior for Unknown, Outdated, and never-starting servers

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/vs/workbench/contrib/mcp/common/mcpGatewayToolBrokerChannel.ts Adds startup grace period mechanism: _startupGrace map, _waitForStartup, and _shouldUseCachedData; updates _listTools, _listResources, _listResourceTemplates to use it
src/vs/workbench/contrib/mcp/test/common/mcpGatewayToolBrokerChannel.test.ts Fixes compile error in createNeverStartingServer; adds createNeverStartingServer helper and three new tests for startup/grace period behavior

You can also share your feedback on Copilot code review. Take the survey.

@connor4312 connor4312 merged commit 5e0dc2f into main Mar 4, 2026
20 checks passed
@connor4312 connor4312 deleted the connor4312/fix-mcp-gateway-compile-error branch March 4, 2026 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants